Skip to content

Remove MediaXMLRPCClient#15178

Merged
wzieba merged 4 commits intotrunkfrom
remove_media_xmlrpc_logic
Jan 20, 2026
Merged

Remove MediaXMLRPCClient#15178
wzieba merged 4 commits intotrunkfrom
remove_media_xmlrpc_logic

Conversation

@wzieba
Copy link
Copy Markdown
Contributor

@wzieba wzieba commented Jan 9, 2026 •
edited
Loading

Closes: AINFRA-1761: Remove MediaXMLRPCClient

Description

Internally (p1767711517542759-slack-CGPNUU63E) we decided that XMLRPC is no longer needed for Media context. This PR removes MediaXMLRPCClient and logs a Sentry event, if the device requested XMLRPC (what shouldn't really happen anymore).

Test Steps

Not needed: if the app builds, we're fine. The removed code was on a dead path.

Images/gif

  • I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

@dangermattic
Copy link
Copy Markdown
Collaborator

dangermattic commented Jan 9, 2026 •
edited
Loading

1 Warning
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

@wzieba wzieba requested a review from Copilot January 9, 2026 12:31
@wpmobilebot
Copy link
Copy Markdown
Collaborator

wpmobilebot commented Jan 9, 2026 •
edited
Loading

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
App NameWooCommerce-Wear Android
Platform⌚️ Wear OS
FlavorJalapeno
Build TypeDebug
Commit74146c2
Direct Downloadwoocommerce-wear-prototype-build-pr15178-74146c2.apk

This comment was marked as outdated.

Sign in to view

@wpmobilebot
Copy link
Copy Markdown
Collaborator

wpmobilebot commented Jan 9, 2026 •
edited
Loading

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App NameWooCommerce Android
Platform📱 Mobile
FlavorJalapeno
Build TypeDebug
Commit74146c2
Direct Downloadwoocommerce-prototype-build-pr15178-74146c2.apk

@wzieba wzieba force-pushed the remove_media_xmlrpc_logic branch 2 times, most recently from cd4a5e7 to fcdc068 Compare January 16, 2026 12:10
@wzieba wzieba added the type: technical debt Represents or solves tech debt of the project. label Jan 16, 2026
@wzieba wzieba added this to the 24.0 milestone Jan 16, 2026
@wzieba wzieba requested a review from Copilot January 16, 2026 12:21
Copilot AI reviewed Jan 16, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

libs/fluxc/src/testFixtures/kotlin/org/wordpress/android/fluxc/wp/site/FakeCrashLogging.kt:1

  • Using println in test fixtures could cause issues in testing environments where stdout is redirected or captured. Consider using a no-op implementation or a list-based implementation that allows tests to verify logged events if needed.
package org.wordpress.android.fluxc.wp.site

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@wzieba wzieba requested a review from Copilot January 16, 2026 13:03
Copilot AI reviewed Jan 16, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/MediaStore.java
mApplicationPasswordsMediaRestClient.uploadMedia(payload.site, payload.media);
} else {
mMediaXmlrpcClient.uploadMedia(payload.site, payload.media);
reportXmlrpcTry();
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reportXmlrpcTry() method only logs to Sentry but doesn't notify an error to the caller. When an upload operation falls into the XMLRPC path, it will silently fail without any error callback being triggered. This should notify an error using notifyMediaUploaded(payload.media, error) with an appropriate MediaError after logging.

Suggested change
reportXmlrpcTry();
reportXmlrpcTry();
notifyMediaUploaded(payload.media, null);

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@wzieba wzieba Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's really necessery to handle - we don't expect this path to be ever executed.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jan 16, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 38.68%. Comparing base (41a3278) to head (74146c2).

Additional details and impacted files 
@@             Coverage Diff              @@
##              trunk   #15178      +/-   ##
============================================
- Coverage     38.68%   38.68%   -0.01%     
- Complexity    10546    10547       +1     
============================================
  Files          2193     2193              
  Lines        124758   124758              
  Branches      17247    17247              
============================================
- Hits          48260    48259       -1     
- Misses        71619    71620       +1     
  Partials       4879     4879

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@wzieba wzieba requested a review from JorgeMucientes January 16, 2026 14:19
@wzieba wzieba marked this pull request as ready for review January 16, 2026 14:19
@wzieba wzieba force-pushed the remove_media_xmlrpc_logic branch from 1d719b1 to 74146c2 Compare January 19, 2026 14:52
@JorgeMucientes JorgeMucientes self-assigned this Jan 19, 2026
JorgeMucientes
JorgeMucientes approved these changes Jan 19, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@JorgeMucientes JorgeMucientes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for surfacing and cleaning this up @wzieba 🙏🏼
LGTM :shipit:

@wzieba wzieba merged commit 9aac6dd into trunk Jan 20, 2026
19 checks passed
@wzieba wzieba deleted the remove_media_xmlrpc_logic branch January 20, 2026 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@JorgeMucientes JorgeMucientes JorgeMucientes approved these changes

Assignees

@JorgeMucientes JorgeMucientes

Labels

type: technical debt Represents or solves tech debt of the project.

Projects

None yet

Milestone

24.0

Development

Successfully merging this pull request may close these issues.

6 participants

@wzieba @dangermattic @wpmobilebot @codecov-commenter @JorgeMucientes